Add APRS packet API, codecs, and vector-test foundation - #3

Review of f535b31

The branch head was rechecked before opening this PR and is unchanged from the reviewed snapshot. This review accepts the design in which intelligible nonconforming traffic is represented with diagnostics and serialization freshly validates structured fields. No code was changed during the review.

All reproductions below use from hamkit import aprs.

1. P1 — Ordinary message serialization can change its meaning into an acknowledgment

Source: _rules.py:399, compared with _codec.py:216.

p = aprs.Packet(aprs.Header("N0CALL", "APRS"), aprs.Message("W1AW", "ack01}02"))
assert aprs.check_packet(p) == ()
wire = aprs.serialize_packet(p)
print(wire)  # b'N0CALL>APRS::W1AW     :ack01}02'
print(aprs.parse_packet(wire).payload)
# Acknowledgment(addressee='W1AW', message_id='01', rejected=False, reply_ack='02')

The constructor guard covers basic ack/rej text but not the decoder's extended reply-ack grammar. rej1} also changes semantics. Match the complete acknowledgment grammar when guarding unnumbered ordinary messages.

2. P1 — Position suffixes bypass extension validation and unsupported handling

Source: _codec.py:197, _codec.py:432.

wire = b"N0CALL>APRS:!4903.50N/07201.75W>999/999Hello"
p = aprs.parse_packet(wire, unsupported="raise")
assert p.diagnostics == ()
assert aprs.serialize_packet(p) == wire

The course/speed-looking suffix is stored entirely as comment text, so course 999 is emitted without a finding. Recognized PHG, altitude, DAO and positioned-weather syntax also enters this comment path without structured interpretation or an unsupported signal. Retaining uninterpreted data for inspection is reasonable, but recognized extension syntax must not bypass fresh send validation. Detect the boundary and validate it or preserve it explicitly as unsupported/raw data.

3. P2 — RF checking disagrees with validation about information length

Source: _codec.py:536, _codec.py:516.

p = aprs.Packet(
    aprs.Header("N0CALL", "APRS"),
    aprs.UserDefined(b"A", b"B", b"x" * 260),
)
assert aprs.check_packet(p, profile="rf") == ()
aprs.validate_packet(p, profile="rf")
# ConformanceError: $.payload: RF information exceeds 256 bytes

The size finding exists only in serialization, despite check_packet promising every conformance finding. Include RF size checking in the shared conformance path for encodable payloads.

4. P2 — Own JSON round trip loses accepted numeric precision

Source: _json.py:339, _validation.py:143.

value = aprs.Calibration(9007199254740993, 0, 0)
restored = aprs.from_json(aprs.to_json(value))
assert restored != value
print(restored.a)  # 9007199254740992.0

Float-annotated fields accept integers, but JSON import unconditionally converts them to float. Preserve accepted numbers or consistently normalize/reject integers that cannot be represented without loss.

5. P2 — Depth limits are applied too late or bypassed

Source: _codec.py:485, _json.py:441, _json.py:284.

p = aprs.Packet(aprs.Header("N0CALL", "APRS"), aprs.Status("x"))
for _ in range(1000):
    p = aprs.Packet(p.header, aprs.ThirdParty(p))

aprs.to_json(p, max_depth=4)  # RecursionError
aprs.serialize_packet(p)    # RecursionError, despite default depth limit 4

Recursive structural validation runs before the serialization depth guard. Bound that traversal before descending.

Recovered JSON has a separate inconsistency:

value = aprs.Malformed(
    b"bad", (aprs.Diagnostic("malformed", "bad"),),
    {"a": {"b": {"c": {"d": {"e": 1}}}}},
)
document = aprs.to_json_dict(value)
aprs.from_json_dict(document, max_depth=3)  # succeeds
aprs.to_json_dict(value, max_depth=3)       # JSONModelError

Import checks the recovered dictionary with an independent fixed limit instead of the caller's remaining depth budget. Apply the requested limit consistently.

6. P2 — Ambiguity bounds lose negative-zero hemispheres

Source: _models.py:95.

p = aprs.Position(-0.0, -0.0, aprs.Symbol("/", "-"), ambiguity=4)
print(p.ambiguity_extent)
# Extent(latitude=(0.0, 1.0), longitude=(0.0, 1.0))

The southern/western cells should be (-1.0, -0.0). Testing value < 0 loses the sign of negative zero; use sign-bit-aware handling, consistent with coordinate serialization.

Documentation and API contract considerations — not blocking bugs

• UTF-8: Supporting UTF-8 in designated free-text fields is reasonable. APRS 1.1 describes 1.2 as a proposed draft; the UTF-8 guidance is dated December 17, 2009. Describe this as adopted compatibility guidance rather than implying a ratified baseline requirement. No ASCII-only mode is requested by this review. The current default emits UTF-8 in free text, including RF mode; monitor/rf selects envelope constraints rather than text encoding. Byte-length probes, ASCII token checks, control-character handling and surrogate rejection behaved correctly. Invalid UTF-8 receives a diagnosed Latin-1 fallback, which is a reversible interpretation rather than proof of the sender's original encoding; original_wire remains the byte evidence.
• Subclass contract: class MyStatus(aprs.Status): pass can be placed in a Packet and wire-serialized, but JSON rejects the subclass through its exact-class registry. Clarify whether subclasses are supported, or align the two acceptance policies.
• The review did not treat stored diagnostics or original_wire as serialization authority, nor object to keyword-only optional fields. Exact original-wire preservation and semantic serialization remain separate contracts.

Validation and limits

• 128 unit tests passed on Python 3.12, including the 20 existing JSON tests.
• mypy passed across 39 files; Black passed across 39 files.
• Ruff reported three findings: __init__.py:112 RUF100, _conformance.py:173 SIM102, and _rules.py:11 I001.
• Remote pre-commit CI passed.
• Python 3.10/3.13 and a fresh wheel build were not independently exercised for this revised head. This is focused review/testing, not exhaustive execution of the documented vector corpus.

Follow-up review of 8567594, with resolution in 0787c90

Status: the three extension findings below were subsequently fixed in 0787c9046c8304d6bcaad161e55c16ce1660b682. They are recorded here as review history, not outstanding findings.

The follow-up reviewed exact head 8567594df7fcc9c1907ccf4cd3f2173f451f1bef, read the PR discussion, and reran the previous reproductions. The diagnostic-bearing packet design remains accepted: preserve intelligible received data, then freshly validate before serialization.

Previous six findings at 8567594

Three extension findings at 8567594 — all resolved

The reproductions below describe the old revision, using from hamkit import aprs as a.

1. P2 — Invalid DF bearing bypassed validation.

Source: _extensions.py:115.

wire = b"N0CALL>APRS:!4903.50N/07201.75W\\088/036/999/729"
p = a.parse_packet(wire, unsupported="raise")
assert p.diagnostics == ()
assert a.serialize_packet(p) == wire

Bearing 999 fell back to comment text without an invalid-extension finding. Resolved: it remains inspectable in the comment, with its original bytes preserved, but gets a nonconforming diagnostic and is refused on send in both monitor and RF modes.

2. P2 — Known zero speed became unavailable.

Source: _extensions.py:198.

p = a.Packet(
    a.Header("N0CALL", "APRS"),
    a.Position(1, 2, a.Symbol("/", "-"),
               extensions=a.Extensions(motion=a.Motion(None, 0))),
)
assert a.check_packet(p) == ()
restored = a.parse_packet(a.serialize_packet(p))
print(restored.payload.extensions.motion)
# Motion(course_deg=None, speed_knots=None, zero_pair='unavailable')

The encoder emitted 000/000, changing a known speed into an unavailable measurement. Resolved: encoding checks that the wire form preserves whether each motion measurement is known and agrees with any explicit zero-pair interpretation. Nonrepresentable combinations are rejected; supported unavailable, fixed-DF, north/zero-speed and unknown-direction/nonzero-speed cases retain focused round-trip tests. Measurement quantization remains supported.

3. P2 — PHG rate was silently discarded.

Source: _extensions.py:233.

p = a.Packet(
    a.Header("N0CALL", "APRS"),
    a.Position(1, 2, a.Symbol("/", "-"),
               extensions=a.Extensions(
                   phg=a.PHG(25, 80, 3, rate_per_hour=5))),
)
assert a.check_packet(p) == ()
print(a.serialize_packet(p))  # ends with PHG5330
restored = a.parse_packet(a.serialize_packet(p))
assert restored.payload.extensions.phg.rate_per_hour is None

Resolved: populated PHG rates, including zero, now raise CodecNotImplementedError on send until their compatibility encoding is implemented. They remain representable in Python and JSON.

The underlying review concern was that the read-back check compared two encoded interpretations, so it could miss fields already dropped by encoding. The fix also explicitly rejects unencoded altitude references and bearing usable/accuracy_less_than_deg annotations rather than silently losing them. This is deliberately limited validation work, not an expansion of codec scope.

Documentation and contract resolutions

• UTF-8 documentation now describes adopted APRS 1.2 proposed-draft compatibility guidance, not a ratified baseline requirement. It explains that the Latin-1 fallback is a reversible reading rather than proof of the sender's encoding, and that monitor/RF selects envelope rules rather than text encoding. No ASCII-only mode was required by this review.
• Subclass handling is documented and consistent: wire serialization, checking and JSON export reject subclass instances.
• Python support intentionally changed to 3.11+ in 8567594.
• The five remaining test import-order findings observed during follow-up were fixed in 0787c90.

Validation of 0787c90

• 139 tests passed from source on Python 3.12.
• A fresh wheel was built and installed into a separate virtual environment; all 139 tests passed against that installed wheel.
• Ruff, Black, mypy and the project's pre-commit hooks all passed.
• Both GitHub workflows reached terminal success: PR CI and push CI.
• The pushed remote SHA was verified. Other Python versions were not independently exercised in this fix pass.
• Planned incomplete codecs remain documented foundation scope; this work does not claim exhaustive APRS compliance or full execution of the documented vector corpus. No merge was performed.

Proposal: separate transport constraints from APRS specification interpretation in the public API.

Use two independent keyword arguments across parsing, checking, and serialization:

• transport=aprs.RF or transport=aprs.APRS_IS
• spec=aprs.APRS_1_1 or spec=aprs.APRS_1_2

This avoids four combined presets. Transport controls accepted address formats, path constraints, and envelope size limits. Spec controls protocol interpretation across the entire packet. It isn’t payload-only: Mic-E also uses the destination address. Both settings can therefore affect the header, for different reasons.

Because APRS 1.2 is a collection of proposals, we should document exactly which behaviors its preset enables, including designated UTF-8 text fields, Mic-E device markers versus legacy telemetry, and expanded telemetry ranges. Invalid-byte fallback should remain an explicit decoding choice. Historical embedded-position searching should also be explicit, rather than presented as a new 1.2 feature.

The defaults and handling of ambiguous legacy packets still need to be decided. This is an API design proposal, not a request to implement it yet.